-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Automatically open the shutters when executing plans #338
Conversation
…`open_shutters_wrapper``.
Tests failing in |
62ce817
to
997cdd4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The configBits for XIA filters/shutters seem convenient for the code logic. However, multiple filters did not work simultaneously. Is it theoretically possible to update this defect in Bluesky or Epics?
from haven.devices import ShutterState | ||
|
||
shutter = filter_bank.shutters[0] | ||
await shutter.set(ShutterState.CLOSED) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we set it to "CLOSED," do filters 2 and 3 close simultaneously or one by one? As Debora and I discussed, the shutter should close simultaneously.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, both filters should move together. The steps are:
- Read the current config bits (e.g. "0001")
- Figure out the new config bits by setting only bits 2 and 3 (.e.g "0010")
- Write the new value to the PV
This way, they should move together.
@@ -85,29 +85,54 @@ def load( | |||
beamline = HavenInstrument( | |||
{ | |||
# Ophyd-async devices | |||
"ion_chamber": IonChamber, | |||
"aerotech_stage": AerotechStage, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this "aerotech_satege" using old electronics or Automation1? Or does it work for anyone similar to Automation1, even if we update some years later?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the old one. I'll get rid of it. We can add new support when Automation1 is ready.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I looked and it should work for both. The fly-scan support is commented-out, so as long as the Automation1 support is just two motors, it doesn't matter. Plus it's already done in ophyd-async so we can just add the fly-scan support when we get there.
return decorator | ||
|
||
|
||
all_decorators = chain( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the "decorators" slow the plan operation speed, making scan time longer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think mostly no, with the exception of the open shutters decorator.
The baseline decorator adds a bunch of reads, but I think they all happen in parallel so whichever the slowest will be added to the scan time.
Shutter suspend decorator just listens for changes in the shutter, so not really.
Open shutters decorator depends on the state of the shutters when the scan starts. 1) If all shutters are open, then the extra time is however long it takes to check if they're open (10 ms?). 2) If a slow shutter is closed, it will add the time needed to open the shutter at the start of the scan and close it at the end (~5s). 3) If a fast shutter is closed, it will add the time needed to open and close the shutter to every point. I measured, and #3 adds about 1s to every point.
We should measure, though.
This one bugged me too, but I'm not sure the right way to fix it. From the comment, I'm thinking you tried something like Right now they use the individual filter open/close signals. These don't work if you set more than one simultaneously, even if you just set the PVs directly so I think it's an IOC question. We could try the config-bits approach for filters too. There's a race condition, though. Let's say the filters have an initial state of "0000" (all open) and we want to close filters 0 and 1. Here are the steps:
Whether this works depends on what filter1 gets in step 4. If step 3 has happened already, then everything is fine, but if step 3 hasn't happened, filter1 gets the old state, updates it and writes the now out of date state back to the device. |
Thanks for the review @yannachen. Merging now. |
Things to do before merging: